Move browser check to the sessions controller#1692
Conversation
The check is unnecessary once users have logged in, and its presence on some unauthenticated pages is blocking things like: - image proxy requests for user avatars (emails) - opengraph requests for public pages (social media) ref: https://app.fizzy.do/5986089/cards/1775 ref: https://app.fizzy.do/5986089/cards/1740
| @@ -1,4 +1,6 @@ | |||
| class SessionsController < ApplicationController | |||
| allow_browser versions: :modern | |||
There was a problem hiding this comment.
I'm not sure if this is the right approach.
People can also transfer their sessions or join through a join code which would skip this controller.
allow_browser should already allow bots through - https://github.com/rails/rails/blob/8aebd0b50738ed8709ae80ef5aa839824a8f2b13/actionpack/lib/action_controller/metal/allow_browser.rb#L98
This might be a bug in UserAgent, or maybe Google changed their bots?
Or maybe Cloudflare blocks the bot?
There was a problem hiding this comment.
UserAgent does not consider the image proxy to be a bot. It's bot check is very rudimentary and the maintainer has resisted expanding it in the couple of PRs I looked at.
Cloudflare is not blocking the bot, I pasted a screenshot of the 406 in one of the cards.
There was a problem hiding this comment.
I put this branch in staging last night and now avatars are working as expected in emails from staging.
|
This is the default location for this setting in Rails. So I'd rather see if there's a way we can fix the check so it allows these bots through. |
|
@dhh there is a 10-year-old pull request open on user agent to add support for google image proxy: The bot check is very rudimentary and not easily extensible by rails without monkeypatching: I will open a pull request upstream with another approach, but at this point I do think we need to do what we need to do for a good launch. |
|
Looks like we've already forked
I guess I'll also try to update our fork, too. |
|
Superseded by #1726 and changes applied to our |
3 participants
The check is unnecessary once users have logged in, and its presence on some unauthenticated pages is blocking things like:
ref: https://app.fizzy.do/5986089/cards/1775
ref: https://app.fizzy.do/5986089/cards/1740
cc @jzimdars